Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix uses of require in goroutines #16953

Merged
merged 4 commits into from
Oct 5, 2022
Merged

Fix uses of require in goroutines #16953

merged 4 commits into from
Oct 5, 2022

Conversation

zmb3
Copy link
Collaborator

@zmb3 zmb3 commented Oct 3, 2022

The require checks from testify call (*testing.T).Fail, which should only be called from the test goroutine.

@github-actions github-actions bot added kubernetes-access tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Oct 3, 2022
@zmb3 zmb3 force-pushed the zmb3/fix-async-requires branch 3 times, most recently from 2a7277b to d0cc23a Compare October 4, 2022 16:09
codingllama
codingllama reviewed Oct 4, 2022
View reviewed changes
Copy link
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you'd rather keep the channels or change other calls to use assert.

api/client/client_test.go Outdated Show resolved Hide resolved
codingllama
codingllama approved these changes Oct 4, 2022
View reviewed changes
lib/backend/firestore/firestorebk_test.go Outdated Show resolved Hide resolved
api/client/client_test.go Outdated Show resolved Hide resolved
@zmb3
Fix uses of require in goroutines
f1574a1 
The require checks from testify call (*testing.T).Fail, which should
only be called from the test goroutine.
@zmb3 zmb3 force-pushed the zmb3/fix-async-requires branch from dc55946 to 12d9010 Compare October 4, 2022 22:34
codingllama
codingllama approved these changes Oct 4, 2022
View reviewed changes
lib/backend/firestore/firestorebk_test.go Outdated Show resolved Hide resolved
@zmb3
Address review comments
12bce1f 
- Restore grpc.Serve() error check
- Combine the stop and channel read into a single cleanup
@zmb3 zmb3 force-pushed the zmb3/fix-async-requires branch from 12d9010 to 12bce1f Compare October 4, 2022 22:57
@zmb3 zmb3 enabled auto-merge (squash) October 5, 2022 01:30
@zmb3 zmb3 disabled auto-merge October 5, 2022 01:34
@zmb3 zmb3 requested a review from rosstimothy October 5, 2022 15:59
@github-actions github-actions bot removed the request for review from alistanis October 5, 2022 16:43
@zmb3 zmb3 enabled auto-merge (squash) October 5, 2022 18:50
@zmb3 zmb3 merged commit 1a62376 into master Oct 5, 2022
@zmb3 zmb3 deleted the zmb3/fix-async-requires branch October 5, 2022 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rosstimothy rosstimothy rosstimothy approved these changes

@codingllama codingllama codingllama approved these changes

Assignees
No one assigned
Labels
kubernetes-access tsh tsh - Teleport's command line tool for logging into nodes running Teleport.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zmb3 @codingllama @rosstimothy